[SYCL] Separated CompileTimePropertiesPass from sycl-post-link tool#7527
Merged
AlexeySachkov merged 28 commits intointel:syclfrom Feb 2, 2023
Merged
Conversation
added 8 commits
November 16, 2022 07:45
DeviceGlobals had to be moved due to the current tight dependency it has to the properties pass. Maybe this could be addressed later?
This is not its final position, just its current one for testing purposes.
…pass They're more appropriate as tests for the pass than the tool.
IR checks would fail/be redundant if kept in sycl-post-link test, so moved them to pass test instead, keeping property checks in old location.
sycl-post-link splits the test into three translation units, if the pass is run ahead of time, the annotations and properties aren't split properly so the pass is run after the split per TU to keep the test working. If the pass can be run before the split, this test can be properly adapted, but this is the only way I can think to preserve this test and maintain coverage as is for now.
added 7 commits
January 6, 2023 05:48
…t were mainly testing pass functionality.
premanandrao
approved these changes
Jan 12, 2023
Contributor
premanandrao
left a comment
premanandrao
left a comment
There was a problem hiding this comment.
FE changes look okay to me.
smanna12
approved these changes
Jan 12, 2023
elizabethandrews
approved these changes
Jan 17, 2023
Fznamznon
reviewed
Jan 19, 2023
AlexeySachkov
suggested changes
Jan 20, 2023
...st/tools/sycl-post-link/device-globals/test_global_variable_many_modules_no_dev_img_scope.ll
Outdated
Show resolved
Hide resolved
v-klochkov
approved these changes
Jan 20, 2023
Contributor
v-klochkov
left a comment
v-klochkov
left a comment
There was a problem hiding this comment.
Ok for ESIMD component.
added 3 commits
January 30, 2023 05:41
The pass moving means these are performed earlier and not in sycl-post-link. This functionality is tested in llvm/test/SYCLLowerIR/CompileTimeProperties/kernel-properties.ll instead.
The test is still required at sycl-post-link splits the source and checks each individually. Putting the opt invocation before the sycl-post-link call will remove the CHECK lines, so this way is necessary.
AlexeySachkov
approved these changes
Jan 31, 2023
added 2 commits
January 31, 2023 08:57
This is to ensure transforms are being applied.
bader
reviewed
Feb 1, 2023
Contributor
bader
left a comment
bader
left a comment
There was a problem hiding this comment.
To be honest, I don't understand the reason to have sycl-post-link passes. I expected that all passes we add for DPC++ should be available for testing via opt tool.
Anyway, this patch looks good to me.
Please, resolve merge conflicts.
Author
|
Current check failure unrelated to PR, failure of some kernel fusion code on OpenCL target. |
Fznamznon
approved these changes
Feb 2, 2023
Contributor
The failure is unrelated and the test is fixed in intel/llvm-test-suite#1557 to match implementation done in #8148 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracted the
CompileTimePropertiespass from thesycl-post-linktool up into thellvmdirectory with the rest of the passes and added it as a step in the code generation pipeline. Moving this into a more generalised location will allow for new tools to be developed that require its logic.Due to a dependency on
DeviceGlobals, that also had to be moved and includes adjusted insycl-post-linktool. This pass can be invoked via opt as well for testing.Several
sycl-post-linktests had to be transferred tollvm/test/SYCLLowerIRwith some being modified as well to account for the change. Some tests had to be split across the two directories so as to maintain coverage. Particular tests that focused on the result of running the pass over some iput bytecode and testing the result have been turned into their own tests under thellvm/test/SYCLLowerIR/CompileTimePropertiesPasstest directory.